Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #2970 add eslint configurations to Docusaurus and play together with turborepo's linting #2971

Merged
merged 3 commits into from
Feb 22, 2022
Merged

Fixes #2970 add eslint configurations to Docusaurus and play together with turborepo's linting #2971

merged 3 commits into from
Feb 22, 2022

Conversation

cindyorangis
Copy link
Contributor

@cindyorangis cindyorangis commented Feb 17, 2022 •
edited
Loading

Issue This PR Addresses

Fixes #2970

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Before when we ran pnpm lint, we get only lint our tree and the Next.js frontend

PS C:\Users\lecin\Documents\repo\telescope> pnpm lint   

> @senecacdot/[email protected] lint C:\Users\lecin\Documents\repo\telescope
> pnpm turbo run lint

• Packages in scope: @senecacdot/autodeployment, @senecacdot/eslint-config-telescope, @senecacdot/feed-discovery-service, @senecacdot/image-service, @senecacdot/planet-service, @senecacdot/posts-service, @senecacdot/search-service, @senecacdot/sso-service, @senecacdot/status-service, @senecacdot/telescope-frontend, dependency-discovery, migrate, parser, telescope-docs
• Running lint in 14 packages
@senecacdot/telescope-frontend:lint: cache hit, replaying output ea6f907d061ff4ab
@senecacdot/telescope-frontend:lint: 
@senecacdot/telescope-frontend:lint: > @senecacdot/[email protected] lint C:\Users\lecin\Documents\repo\telescope\src\web
@senecacdot/telescope-frontend:lint: > pnpm eslint
@senecacdot/telescope-frontend:lint: 

 Tasks:    1 successful, 1 total
Cached:    1 cached, 1 total
  Time:    1.254s >>> FULL TURBO

With this PR, we will also lint the Docusaurus sub-project

PS C:\Users\lecin\Documents\repo\telescope> pnpm lint

> @senecacdot/[email protected] lint C:\Users\lecin\Documents\repo\telescope
> pnpm turbo run lint

• Packages in scope: @senecacdot/autodeployment, @senecacdot/eslint-config-telescope, @senecacdot/feed-discovery-service, @senecacdot/image-service, @senecacdot/planet-service, @senecacdot/posts-service, @senecacdot/search-service, @senecacdot/sso-service, @senecacdot/status-service, @senecacdot/telescope-docs, @senecacdot/telescope-frontend, dependency-discovery, migrate, parser
• Running lint in 14 packages
@senecacdot/telescope-frontend:lint: cache hit, replaying output 7aa4bbab10a81473
@senecacdot/telescope-frontend:lint: 
@senecacdot/telescope-docs:lint: cache hit, replaying output 068f9f9922d75b3e
@senecacdot/telescope-docs:lint: 
@senecacdot/telescope-docs:lint: > @senecacdot/[email protected] lint C:\Users\lecin\Documents\repo\telescope\src\docs
@senecacdot/telescope-docs:lint: > pnpm eslint
@senecacdot/telescope-docs:lint: 
@senecacdot/telescope-frontend:lint: > @senecacdot/[email protected] lint C:\Users\lecin\Documents\repo\telescope\src\web
@senecacdot/telescope-frontend:lint: > pnpm eslint
@senecacdot/telescope-frontend:lint: 

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    1.284s >>> FULL TURBO

Steps to test the PR

To test this PR locally:

  1. Add my repository as a remote: git remote add cindyledev https://github.com/cindyledev/telescope.git
  2. Fetch my branches: git fetch cindyledev
  3. Checkout my branch: git checkout issue-2970
  4. Install dependencies using pnpm in root (this will also install dependencies in src/docs: pnpm install
  5. Lint: pnpm lint

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@cindyorangis cindyorangis added the area: docusaurus Anything related to Docusaurus label Feb 17, 2022
@cindyorangis cindyorangis added this to the 2.8 Release milestone Feb 17, 2022
@cindyorangis cindyorangis self-assigned this Feb 17, 2022
@gitpod-io
Copy link

gitpod-io bot commented Feb 17, 2022

@cindyorangis cindyorangis requested a review from menghif February 19, 2022 19:35
menghif
menghif reviewed Feb 21, 2022
View reviewed changes
tools/eslint/package.json
@@ -14,6 +14,7 @@
"eslint-plugin-jest": "25.3.4",
"eslint-plugin-jest-playwright": "0.2.1",
"eslint-plugin-jsx-a11y": "6.5.1",
"eslint-plugin-node": "11.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In PR #2853, we added a number of eslint overrides, and they addressed all the eslint errors we were getting for the Docusaurus app. Hence, the overrides in this PR are identical to what we had previously (just located in src/docs as opposed to /).

For some reason, I am no longer getting any eslint errors for the Docusaurus app on master right now. I don't know if this PR is even needed...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you introduce an eslint error, does it catch that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@menghif we use eslint-plugin-node here

telescope/src/docs/.eslintrc.js

Lines 25 to 26 in 36f2307

'node/no-missing-import': 'off',
'node/no-unsupported-features/es-syntax': 'off',

or else we get errors like these
2022-02-22 13_24_38-Window

humphd
humphd previously approved these changes Feb 22, 2022
View reviewed changes
@humphd humphd requested a review from tcvan0707 February 22, 2022 21:25
tcvan0707
tcvan0707 previously approved these changes Feb 22, 2022
View reviewed changes
@cindyorangis cindyorangis dismissed stale reviews from tcvan0707 and humphd via 102758e February 22, 2022 22:17
@cindyorangis cindyorangis merged commit 298f5ae into Seneca-CDOT:master Feb 22, 2022
@aserputov
Copy link
Contributor

aserputov commented Feb 22, 2022 •
edited
Loading

@cindyledev we have three commits here.🤭 It's better to rebase I think.

@cindyorangis
Copy link
Contributor Author

@cindyledev we have three commits here.🤭 It's better to rebase I think.

three commits here but only one commit was merged into master ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@menghif menghif menghif left review comments

@tcvan0707 tcvan0707 tcvan0707 left review comments

@humphd humphd humphd left review comments

Assignees

@cindyorangis cindyorangis

Labels
area: docusaurus Anything related to Docusaurus
Projects
None yet
Milestone
2.8 Release
Development

Successfully merging this pull request may close these issues.

Configure ESLint for Docusaurus sub-project
5 participants
@cindyorangis @aserputov @humphd @menghif @tcvan0707